Skip to content

tooling(hygiene): --files arg on check-tick-history-shard-schema.sh — pre-push compat (stacked on #975)#977

Merged
AceHack merged 2 commits intomainfrom
tooling/check-tick-history-shard-schema-files-arg-stacked
Apr 30, 2026
Merged

tooling(hygiene): --files arg on check-tick-history-shard-schema.sh — pre-push compat (stacked on #975)#977
AceHack merged 2 commits intomainfrom
tooling/check-tick-history-shard-schema-files-arg-stacked

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented Apr 30, 2026

Summary

Stacks on #975 (DORMANT-mode landing of the schema check). Adds a --files PATH... argument so the check can run on a restricted set of shards instead of the full tree.

This PR's branch base is tooling/check-tick-history-shard-schema-prevent-col1-drift-2026-04-30 (#975's branch), so its diff against main contains BOTH #975's content AND this refactor. When #975 merges first, this PR becomes a clean +107/-52 diff against the new main; when #975 + this both auto-merge, the second merge is a no-op for #975's content.

Why this matters

The default full-tree mode currently fails on 5 known-stale April-28 shards (#975 lands DORMANT for that reason). A pre-push hook running the full-tree check would always fail until those 5 are resolved — and they can't be mechanically resolved because of the prefab finding (#973).

The --files mode sidesteps the deadlock: pre-push hooks pass it the list of shards changed in the current diff, and the check scans only those. New violations caught at write-time; the 5 known-stale shards stay quietly violating until the prefab decision unsticks them.

Behavior

Mode Scans Use case
(no args) All shards under docs/hygiene-history/ticks/ Manual runs, full-tree audits
--files PATH... Only the listed shard paths (non-shard paths silently skipped) Pre-push hooks, per-PR CI

Composes with

Test plan

🤖 Generated with Claude Code

…hema.sh — pre-push compat

Default mode (full-tree) is unchanged — manual runs and full-
tree audits work as before.

New --files PATH... mode restricts the check to the listed
shard files. Shape that pre-push hooks and per-PR CI jobs
want, so they can run only on changed shards instead of
failing on the 5 known-stale shards documented in the script
header. Non-shard paths in the argument list are silently
skipped, so callers can pass a broader file list (e.g. all
changed files from `git diff --name-only`).

Refactor: extracted the per-shard validator into scan_one()
function; replaced `continue` with `return 1` for early-exit
semantics that work both inside the find loop and inside the
--files iteration.

Composes with B-0114 sub-item 1 (pre-push lint hook) — the
--files mode is what that hook will invoke.

Stacks on #975 (the original DORMANT-mode landing).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@AceHack AceHack force-pushed the tooling/check-tick-history-shard-schema-files-arg-stacked branch from 92c73a4 to f708a61 Compare April 30, 2026 23:39
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92c73a4623

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/hygiene/check-tick-history-shard-schema.sh
Comment thread tools/hygiene/check-tick-history-shard-schema.sh Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a hygiene checker script for tick-history shard files and extends it with a --files PATH... mode intended for pre-push / per-PR CI runs that only validate changed shards (avoiding known historical violations in full-tree mode).

Changes:

  • Introduces tools/hygiene/check-tick-history-shard-schema.sh to validate shard filename + col1 timestamp schema and ensure timestamp matches the shard’s directory date + filename HHMM.
  • Adds --files mode to restrict scanning to a caller-provided list of shard paths (skipping non-shard paths).

Comment thread tools/hygiene/check-tick-history-shard-schema.sh
Comment thread tools/hygiene/check-tick-history-shard-schema.sh
Comment thread tools/hygiene/check-tick-history-shard-schema.sh Outdated
…review

Three substantive findings from the chatgpt-codex-connector +
copilot-pull-request-reviewer review pass on PR #977:

1. P2 (Codex) — six-column row enforcement. The col1 regex
   only validated col1 + the col1-end pipe; rows with too few
   columns (e.g. `| <ts> | a |`) passed. Added explicit pipe-
   count check via awk that requires ≥7 pipes (= 6 columns +
   trailing pipe). Runs before the col1 regex; col1 check
   only fires if column count is right.

2. P2 (Codex) — HHMMSSZ filenames must require the hash
   suffix. The `(-[0-9a-f]+)?$` made the suffix optional,
   so `111122Z.md` would pass even though the schema documents
   `HHMMSSZ-<hash>.md` with hash REQUIRED for collision-
   avoidance under high-concurrency writes. Made the hash
   non-optional in the HHMMSS branch.

3. P2 (Copilot) — header contract said non-shard paths "silently
   skipped" but the code emitted `skipped (not a file): ...`
   to stderr. Removed the stderr message to match the contract.

Two findings left as form-2 closures (false positives):

- P0 (Copilot) — case glob doesn't span YYYY/MM/DD depth.
  False: bash case patterns `*` matches `/`. Verified by
  running --files with a real 4-level path; matched
  correctly. Added a comment explaining the bash-case-glob
  semantics so future readers don't re-flag.

- P0 (Copilot) — col1 regex requires "two spaces after |".
  False: trailing whitespace before `]]` is regex delimiter,
  not part of the pattern. Verified the regex matches single-
  space shards (the actual on-main format).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@AceHack AceHack merged commit e3af21f into main Apr 30, 2026
24 checks passed
@AceHack AceHack deleted the tooling/check-tick-history-shard-schema-files-arg-stacked branch April 30, 2026 23:53
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e6392f100

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pipe_count=$(awk -F'|' '{print NF-1}' <<< "$first_line")
# 6 columns => 7 pipes (one before col1, one between each pair, one
# after col6). Allow 7 or 8 to tolerate trailing whitespace.
if [ "$pipe_count" -lt 7 ]; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject first rows with more than six columns

The new row-shape guard only checks pipe_count < 7, so rows with extra delimiters still pass even though docs/hygiene-history/ticks/README.md defines a fixed 6-column schema. In --files mode this yields false negatives for malformed shards (for example a body cell containing an unescaped | produces 8+ pipes and exits 0), so pre-push/CI can accept structurally invalid shard rows. Tightening this to enforce the exact table width (or a parser that distinguishes escaped pipes) is needed for the check to actually enforce the declared schema.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants